fix: Dragging applications in application folder cannot create a new page by wjyrich · Pull Request #558 · linuxdeepin/dde-launchpad

wjyrich · 2025-05-13T00:36:11Z

as title.

pms-bug-288839

Summary by Sourcery

Enable dynamic creation of new pages within application folders by dragging items to the folder’s edge and update the page indicator when the folder’s page count changes.

Bug Fixes:

Allow dragging applications to the right padding area of an application folder to create a new page instead of blocking the action.

Enhancements:

Introduce a createdEmptyPage flag in QML to prevent duplicate page creation during drag operations.
Extend ItemArrangementProxyModel::creatEmptyPage to accept a folder ID and handle folder-specific page creation.
Emit folderPageCountChanged from the proxy model and update the QML gridViews repeater model when a folder’s page count changes.
Reset the createdEmptyPage flag after completing a drop operation.

deepin-ci-robot · 2025-05-13T00:36:33Z

deepin pr auto review

代码审查意见：

在 FolderGridViewPopup.qml 文件中，新增的 createdEmptyPage 属性应该添加注释说明其用途和作用范围。
在 checkDragMove 函数中，isLastPage 变量应该添加注释说明其含义。
在 onTriggered 信号处理函数中，isLastPage 变量和 folderPageDropArea.createdEmptyPage 的逻辑判断应该添加注释说明其目的。
在 ItemArrangementProxyModel 类的 creatEmptyPage 函数中，参数 folderId 应该添加注释说明其用途。
在 ItemArrangementProxyModel 类的 creatEmptyPage 函数中，添加了对 folderId 的检查，确保只有在 folderId 有效时才创建空页面。
在 ItemArrangementProxyModel 类的 creatEmptyPage 函数中，添加了 qWarning 输出，以便在创建空页面失败时提供调试信息。
在 ItemArrangementProxyModel 类的 createFolder 函数中，添加了对 page 对象的 pageCountChanged 信号的连接，以便在页面计数变化时更新文件夹页面计数。
在 ItemArrangementProxyModel 类的 createFolder 函数中，folderId 的提取应该使用 QStringView 以提高性能。
在 ItemArrangementProxyModel 类的 createFolder 函数中，connect 语句应该添加注释说明其目的和作用。
在 ItemArrangementProxyModel 类的 creatEmptyPage 函数中，m_topLevel->appendEmptyPage(); 应该检查 m_topLevel 是否为空，以避免潜在的空指针异常。
在 ItemArrangementProxyModel 类的 creatEmptyPage 函数中，m_folders.contains(fullId) 的检查应该添加注释说明其目的。
在 ItemArrangementProxyModel 类的 creatEmptyPage 函数中，itemPage->appendEmptyPage(); 应该检查 itemPage 是否为空，以避免潜在的空指针异常。
在 ItemArrangementProxyModel 类的 creatEmptyPage 函数中，qWarning 输出的消息应该包含更多的上下文信息，以便更容易地定位问题。
在 ItemArrangementProxyModel 类的 creatEmptyPage 函数中，return 0; 应该添加注释说明其含义，因为返回 0 可能会导致逻辑错误。
在 ItemArrangementProxyModel 类的 creatEmptyPage 函数中，Q_ASSERT 应该添加注释说明其目的，因为 Q_ASSERT 通常用于调试目的。
在 ItemArrangementProxyModel 类的 creatEmptyPage 函数中，Q_ASSERT 应该替换为更合适的错误处理机制，因为 Q_ASSERT 在发布版本中可能会被禁用。
在 ItemArrangementProxyModel 类的 creatEmptyPage 函数中，Q_ASSERT 应该检查 folderId 是否有效，因为无效的 folderId 可能会导致逻辑错误。
在 ItemArrangementProxyModel 类的 creatEmptyPage 函数中，Q_ASSERT 应该检查 m_folders 是否为空，因为空的 m_folders 可能会导致逻辑错误。
在 ItemArrangementProxyModel 类的 creatEmptyPage 函数中，Q_ASSERT 应该检查 itemPage 是否为空，因为空的 itemPage 可能会导致逻辑错误。
在 ItemArrangementProxyModel 类的 creatEmptyPage 函数中，Q_ASSERT 应该检查 itemPage->appendEmptyPage() 的返回值，因为 appendEmptyPage() 可能会失败。

sourcery-ai · 2025-05-13T00:37:48Z

Reviewer's Guide

This PR enhances the folder drag-and-drop experience by creating new empty pages on demand when dragging past the last page in a folder and propagates folder-specific page count updates to the QML view.

File-Level Changes

Change	Details	Files
Implement on-demand empty page creation in folder drag flow	Add createdEmptyPage flag to FolderGridViewPopup Guard checkDragMove to prevent duplicate page creation Reset createdEmptyPage after a drop event Extend auto-scroll handler to invoke creatEmptyPage and navigate to the new page	`qml/FolderGridViewPopup.qml`
Extend creatEmptyPage API to support folder context	Change creatEmptyPage signature to take an optional folderId parameter Implement logic to append an empty page for the specified folder or top-level when folderId is zero Return appropriate new page index or fallback on failure	`src/models/itemarrangementproxymodel.cpp` `src/models/itemarrangementproxymodel.h`
Propagate folder-specific page count changes to QML	Emit folderPageCountChanged(int) in ItemArrangementProxyModel when an ItemsPage’s count changes Add Connections block in FolderGridViewPopup.qml to refresh gridViews.model on folderPageCountChanged Connect ItemsPage::pageCountChanged to emit folderPageCountChanged in createFolder	`src/models/itemarrangementproxymodel.cpp` `src/models/itemarrangementproxymodel.h` `qml/FolderGridViewPopup.qml`

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey @wjyrich - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟡 General issues: 4 issues found
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

sourcery-ai · 2025-05-13T00:43:02Z

    Q_INVOKABLE void bringToFront(const QString & id);
    Q_INVOKABLE void commitDndOperation(const QString & dragId, const QString & dropId, const DndOperation op, int pageHint = -1);
-    Q_INVOKABLE int creatEmptyPage() const;
+    Q_INVOKABLE int creatEmptyPage(int folderId = 0) const;


suggestion: Method name typo and const correctness

Rename creatEmptyPage to createEmptyPage for clarity, and drop the const qualifier since this method mutates internal state.

sourcery-ai · 2025-05-13T00:43:02Z

+        return m_topLevel->pageCount() - 1;
+    }
+    QString fullId("internal/folders/" + QString::number(folderId));
+    Q_ASSERT(m_folders.contains(fullId));


suggestion: Using Q_ASSERT here may abort in debug builds

Replace the assert with a runtime check—since you already warn and return on missing folders—to avoid debug-only aborts and keep behavior consistent.

Suggested change

Q_ASSERT(m_folders.contains(fullId));

if (!m_folders.contains(fullId)) {

qWarning() << "Folder not found, returning 0. fullId is" << fullId;

return 0;

}

sourcery-ai · 2025-05-13T00:43:02Z

    m_folderModel.appendRow(folder);

+    connect(page, &ItemsPage::pageCountChanged, this, [this, fullId]() {
+        int folderId = QStringView{fullId}.mid(17).toInt();


suggestion: Hard-coded index for parsing folderId is brittle

Store the numeric ID separately or derive it with split/regex rather than relying on a hard-coded offset.

…new page as title. pms-bug-288839

deepin-ci-robot · 2025-05-13T05:17:57Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, wjyrich

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

sourcery-ai Bot reviewed May 13, 2025

View reviewed changes

fix: Dragging applications in the application folder cannot create a …

a00c539

…new page as title. pms-bug-288839

wjyrich force-pushed the fix-bug-288839 branch from c88df9d to a00c539 Compare May 13, 2025 02:36

BLumia approved these changes May 13, 2025

View reviewed changes

BLumia merged commit 13160be into linuxdeepin:master May 13, 2025
7 of 10 checks passed

wjyrich deleted the fix-bug-288839 branch June 6, 2025 00:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Dragging applications in application folder cannot create a new page#558

fix: Dragging applications in application folder cannot create a new page#558
BLumia merged 1 commit into
linuxdeepin:masterfrom
wjyrich:fix-bug-288839

wjyrich commented May 13, 2025 •

edited by sourcery-ai Bot

Loading

Uh oh!

deepin-ci-robot commented May 13, 2025

Uh oh!

sourcery-ai Bot commented May 13, 2025

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai Bot left a comment

Uh oh!

Uh oh!

sourcery-ai Bot May 13, 2025

Uh oh!

sourcery-ai Bot May 13, 2025

Uh oh!

sourcery-ai Bot May 13, 2025

Uh oh!

deepin-ci-robot commented May 13, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-    Q_ASSERT(m_folders.contains(fullId));
+    if (!m_folders.contains(fullId)) {
+        qWarning() << "Folder not found, returning 0. fullId is" << fullId;
+        return 0;
+    }

Conversation

wjyrich commented May 13, 2025 • edited by sourcery-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Uh oh!

deepin-ci-robot commented May 13, 2025

deepin pr auto review

Uh oh!

sourcery-ai Bot commented May 13, 2025

Reviewer's Guide

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sourcery-ai Bot May 13, 2025

Choose a reason for hiding this comment

Uh oh!

sourcery-ai Bot May 13, 2025

Choose a reason for hiding this comment

Uh oh!

sourcery-ai Bot May 13, 2025

Choose a reason for hiding this comment

Uh oh!

deepin-ci-robot commented May 13, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wjyrich commented May 13, 2025 •

edited by sourcery-ai Bot

Loading